-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨ (signer-btc) [DSDK-471]: SignPsbt task #581
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
9cce5af
to
47b61b5
Compare
0034834
to
7182bae
Compare
7182bae
to
76bdcfd
Compare
f08eb42
to
c922a05
Compare
c922a05
to
0663f38
Compare
}, | ||
initialValues: { | ||
derivationPath: DEFAULT_DERIVATION_PATH, | ||
psbt: "70736274ff0104010101fb0402000000010204020000000105010100011004000000000103040100000001007102000000013daeeb9a92e7b5af90c787d53f0e60d2cf4cfd47bca9a0d8bc77a7464b024c0b00000000000000000002ff0300000000000016001402fe597c6ec0e2982712929bcf079a4e11d37e8d950b0000000000001600144dc432cb6a26c52a1e6ddd2bcf0ee49199fae0cc000000002206031869567d5e88d988ff7baf6827983f89530ddd79dbaeadaa6ec538a8f03dea8b18f5acc2fd540000800000008000000080000000000000000001011fff0300000000000016001402fe597c6ec0e2982712929bcf079a4e11d37e8d010e200cf08d04fa11ff024d5a50165ba65e495409b50ba6657788dfa15274adb682df010f0400000000000103086b01000000000000010416001429159115f12bb6a7e977439c83d3f8d555d72d5f00", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[COULD] Remove the default psbt here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, it's a heartache after so much difficulties to create it 😿
|
||
export type SignPsbtDAInternalState = { | ||
readonly error: SignPsbtDAError | null; | ||
// [SHOULD] be psbt instead of signature |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[ASK] What to do with this comment ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The next task I'm working on, UpdatePsbtTask, will modify this device action to return a psbt instead of a Signature
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually as discussed, we won't return a PSBT but the list of signature, with according index/tag/pubkey_augm (typing to be refined)
{ | ||
constructor(private readonly args: GetWalletAddressCommandArgs) {} | ||
constructor( | ||
private readonly args: GetWalletAddressCommandArgs, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[COULD] rename args
to _args
@@ -41,19 +37,26 @@ export type SignMessageCommandArgs = { | |||
readonly messageMerkleRoot: Uint8Array; | |||
}; | |||
|
|||
export type SignMessageCommandResponse = Signature | ApduResponse; | |||
export type SignMessageCommandResponse = ApduResponse; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[ASK] Why returning an ApduResponse
instead of a Signature
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To handle the signature only in the task, with a client interpreter flow as the it should respond with those client commands.
packages/signer/signer-btc/src/internal/use-cases/sign-psbt/SignPsbtUseCase.ts
Show resolved
Hide resolved
02dd40e
to
f9c201f
Compare
f9c201f
to
4f01dfe
Compare
packages/signer/signer-btc/src/internal/app-binder/task/SignPsbtTask.ts
Outdated
Show resolved
Hide resolved
9af8aea
to
cfdf8dd
Compare
cfdf8dd
to
242b0c1
Compare
8bdffc0
to
118746b
Compare
packages/signer/signer-btc/src/internal/app-binder/task/SignPsbtTask.ts
Outdated
Show resolved
Hide resolved
packages/signer/signer-btc/src/internal/app-binder/task/SignPsbtTask.ts
Outdated
Show resolved
Hide resolved
packages/signer/signer-btc/src/internal/app-binder/task/SignPsbtTask.ts
Outdated
Show resolved
Hide resolved
286dfae
to
fe39d19
Compare
packages/signer/signer-btc/src/internal/app-binder/task/SignPsbtTask.ts
Outdated
Show resolved
Hide resolved
packages/signer/signer-btc/src/internal/app-binder/task/SignPsbtTask.ts
Outdated
Show resolved
Hide resolved
fe39d19
to
1fdb8fa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
📝 Description
sign_psbt.mp4
❓ Context
✅ Checklist
Pull Requests must pass CI checks and undergo code review. Set the PR as Draft if it is not yet ready for review.
🧐 Checklist for the PR Reviewers